docs: add the org Integrations UI walkthrough for webhooks [OD-702] - #2758
claudiacodacy wants to merge 1 commit into
Conversation
|
Overall readability score: 54.21 (🟢 +0)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
The webhook verification documentation allows replay of captured valid deliveries because the timestamp and delivery ID are not covered by the signature and replay handling is not defined. This security gap should be addressed before merging.
The strict MkDocs build and several acceptance criteria remain unverified because no build artifact or automated test evidence is included. Codacy is up to standards; no uncovered complex files were reported.
Test suggestions
- Strict MkDocs build validates the new page and navigation entry without warnings.
- Documentation covers adding, deleting, endpoint limits, permissions, HTTPS validation, and signing-secret lifecycle.
- Documentation accurately describes branch and pull request event payloads and delivery conditions.
- Documentation accurately describes headers, HMAC-SHA256 verification, timeout, retry, and deduplication behavior.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Strict MkDocs build validates the new page and navigation entry without warnings.
2. Documentation covers adding, deleting, endpoint limits, permissions, HTTPS validation, and signing-secret lifecycle.
3. Documentation accurately describes branch and pull request event payloads and delivery conditions.
4. Documentation accurately describes headers, HMAC-SHA256 verification, timeout, retry, and deduplication behavior.
Low confidence findings
- Validate the documented endpoint-management flow against the shipped UI before relying on it as the authoritative guide.
- Add or link automated evidence that
mkdocs build --strictcompletes without warnings.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
|
||
| 1. Compute the HMAC-SHA256 hash of the raw request body, using the endpoint's signing secret as the key. | ||
| 1. Hex-encode the hash and prefix it with `sha256=`. | ||
| 1. Compare the result to the `X-Codacy-Signature` header using a constant-time comparison, and reject the delivery if they don't match. |
There was a problem hiding this comment.
🟡 MEDIUM RISK
This verification flow permits replay of a captured, valid delivery. Include the timestamp and delivery ID in the signed material, or explicitly require consumers to deduplicate X-Codacy-Delivery values and document that the timestamp cannot be trusted for freshness unless it is covered by the signature. Define the exact HMAC input, require constant-time signature comparison, and explain rejection of duplicate delivery IDs and stale requests.
|
Heads-up: the M1 wire contract changed after this was approved (outbound-hooks#15, codacy-events#260):
|
|
amazing |
|
In "Delivery behavior", please also note that Codacy retries a 5xx or a timeout up to 2 more times (200 ms, 400 ms delay) before giving up; only a 4xx drops immediately, without retry. A retry reuses the same |
22e08dd to
3411b56
Compare
claudiacodacy
changed the title
|
Addressed both, on #2767 (the wire-contract content, inherited here):
Also split this PR in two: #2767 ships the API-only page this week (org Integrations UI isn't built yet), and this PR now carries just the UI walkthrough on top, to merge once the UI ships. |
|
Update to the retry timing (outbound-hooks#24): a 5xx or timeout is now retried with exponential backoff, about 1 s then 2 s (with jitter), still 3 attempts in total; a 4xx is not retried. Please use these numbers instead of 200 ms / 400 ms. |
3411b56 to
694e068
Compare
8d9b516 to
5d8fb7e
Compare
|
Correction to my previous note: the retry delays are about 1 s then 5 s (exponential factor 5, with jitter), still 3 attempts in total; a 4xx is not retried. |
Restacks on #2767 (the API-only release for this week) and adds the org Integrations > Webhooks UI: the Add endpoint flow, the one-time signing-secret card, the endpoint list, and the upgrade prompt shown when the organization isn't entitled. Merge once the UI ships (OD-697, OD-699, OD-701, OD-709). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
694e068 to
0afd385
Compare
This branch was successfully deployed
3 participants
Summary
This is the second half of the original webhooks documentation PR, split so we can ship the API-only page this week (see #2767) while the org Integrations UI (OD-697, OD-699, OD-701, OD-709) is still being built. Merge this once the UI ships, after #2767 is merged into
masterand this branch is rebased onto it.Review history
The two wire-contract corrections raised in review on the original PR are already applied (on #2767, inherited here):
X-Codacy-Timestamp, movedtimestampinto the signed body as ISO 8601, added thestatusfield.5xx/timeout retries up to 2 more times at 200 ms/400 ms,4xxdrops immediately, retries reuseX-Codacy-Delivery).Test plan
mkdocs build --strictpasses with no warningsvale docs/organizations/integrations/webhooks.md— clean except the pre-existing repo-wide em dash spacing style (Microsoft.Dashes), advisory🤖 Generated with Claude Code